Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_HAL_ChibiOS: Fix waf --default-parameters #13512

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

Pedals2Paddles
Copy link
Contributor

If a defaults.parm file was present in the hwdef, waf configure ignored the --default-parameters command line argument. It also treated any specified file as it's root being the repo directory, which is not very useful. It would turn an absolute path into something ridiculous like /home/matt/github/ardupilot/home/matt/Desktop/paramfile.params which is clearly invalid.

This PR does a few things to fix all this.

  • Allow you to specify and absolute path to a parameter file that works, looking like this: ./waf configure CubeOrange --default-parameters=/home/matt/Desktop/paramfile.params.
  • A parameter file specified with the --default-parameters argument will take precedent over a defaults.parm file in the hwdef directory.
  • Additional prints to tell the user what it actually is doing with what files.

It should be noted, which I didn't know until this, that this command line argument is only used in configure. Not in the build command.

If a defaults.parm file was present in the hwdef, waf ignored the --default-parameters=xyz.parm command line argument.  This will allow it to use that command line argument specified file.
@Pedals2Paddles
Copy link
Contributor Author

@peterbarker @tridge

@Pedals2Paddles
Copy link
Contributor Author

I'm wondering if it might be a good idea to append the --default-parameters specified file to the end of the defaults.parm file if present. That was those defaults in the defaults.parm are maintained since they may be rather important.

@WickedShell
Copy link
Contributor

@Pedals2Paddles that would have been my assumption for how that would work actually, but I could see the debate either way.

@Pedals2Paddles
Copy link
Contributor Author

On second thought, I'm not sure I want to do that. The user specified a set of params. I can't make a new combined file, because now it's something else the user doesn't even have. And I can't append or prepend without totally hosing the users files. So I think it needs to just be the users file or not.

@Pedals2Paddles
Copy link
Contributor Author

I've been using this all weekend with no defaults, hwdef defaults, and command line specified defaults. It's working splendid.

@tridge tridge merged commit e6f32f4 into ArduPilot:master Feb 11, 2020
@tridge
Copy link
Contributor

tridge commented Feb 11, 2020

nice work, thanks!

@Pedals2Paddles
Copy link
Contributor Author

Sorry I missed this on the dev call. Thanks Tridge!

@Pedals2Paddles Pedals2Paddles deleted the waf-param-fix branch February 12, 2020 02:03
@rmackay9
Copy link
Contributor

rmackay9 commented May 6, 2020

this is included in Copter-4.0.4-rc1

@rmackay9 rmackay9 moved this from PRs to 4.0.4-rc1 in Copter 4.0 backports May 6, 2020
@trexgerratt
Copy link

Sorry to comment on such an old post, but I am having trouble getting default params flashed to my board. I've tried both a Pixhawk 4 and a mRoControlZero. I've tried various different commands/approaches without success. Here's what I think I should be running:
./waf configure --board mRoControlZeroOEMH7 --default-parameters=/home/ubuntu/ardupilot_params/defaults.parm
./waf copter --upload

During the build, I see it says "Setting defaults from /home/ubuntu/ardupilot_params/defaults.parm" so it appears the command is working, however, the params on the autopilot do not reflect what are stored in my file.

I've tried comma-separated, space-separated, even adding param defaults to the "hwdef/mRoControlZeroOEMH7/defaults.parm" file. Most of my testing has been on the latest version of ardupilot, but I also checked out this commit (e6f32f4) with no success.

Am I missing something? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants